-
Notifications
You must be signed in to change notification settings - Fork 638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NTP: use sntp app from lwip on latest Cores, replace ntpclientlib #2132
Conversation
|
Nice job 👍🏻 |
None, really. Re-enabling scheduler code, travis shows the same size for basic.h build. |
In general Not really looking too portable, since there needs to be a code chunk using libc functions instead of timelib. I guess it can temporarily |
… reinit for some reason)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timelib helpers are working, scheduler days are in order.
Everything builds even with lib_deps
referencing NtpClientLib, only a matter of reading platformio documentation more closely on how it parses dependencies.
if (cfg_tz != active_tz) { | ||
setenv("TZ", cfg_tz.c_str(), 1); | ||
tzset(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since configTime re-inits sntp, set timezone through tzset(3)
DEBUG_MSG_P(PSTR("[NTP] Updating `ntpServer` setting from DHCP: %s\n"), server.c_str()); | ||
_ntp_server = server; | ||
setSetting("ntpServer", server); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dhcp routine will call sntp_init() whenever dhcp server sets ntp option:
#1828
I also wonder if this is the reason that sntp can never be optimised away
// TODO: | ||
// terminalRegisterCommand(F("NTP.SYNC"), [](Embedis* e) { ... } | ||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't exactly speed-up sntp like NtpClientLib, so idk about this.
Changing ntpServer
value also triggers sync
// TODO: need this prototype for .ino | ||
struct NtpCalendarWeekday; | ||
|
||
#if NTP_SUPPORT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW some benefit of having #if
guards in headers is that compilation will fail instead of linker when function is not implemented / supported
I am currently using 1.14.1 and ntp fails to get time from the internet, Is there any solution available to make it work with current version or I need to wait for 1.14.2 to get released? |
download the |
subj
fix #1380 (yay)
fix #1517
fix #1629
TODO:
slightly complicated because of restore-last-schedule since rpn is generic and scheduler part would need to iterate over $tick rules. Can use a different rpn context and ruleset / settings, maybe.
ntplegacyVisible
hint to hide offending parts or we could also have a separate panel.